Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: the bug of blobsidecars and downloader with multi-database #2564

Merged
merged 18 commits into from
Jul 16, 2024
Merged

fix: the bug of blobsidecars and downloader with multi-database #2564

merged 18 commits into from
Jul 16, 2024

Conversation

jingjunLi
Copy link
Contributor

@jingjunLi jingjunLi commented Jul 4, 2024 •
edited
Loading

fix: some bugs for multi-database

Description

Rationale

Issue:

  1. Fixed the issue with force kill when using blob sidecars.
  2. Fixed the issue with "Failed to decode block body."
  3. Fixed the issue where some Ancient-related interface calls did not specify BlockStore() under multi-database.
  4. Fixed the issue where state-related data was not being written to stateStore in snap sync mode.

Improvement:

  1. The multidatabase flag is now only used by the init command.
  2. Refactored block meta-related interfaces, adding the MultiDatabaseReader interface. This allows reading meta interfaces without specifying blockStore, while Write/Delete related interfaces still require it.

Example

add an example CLI or API response...

Changes

Notable changes:

  • add each change in a bullet point here
  • ...

@jingjunLi jingjunLi changed the title fix: fix api for ReadBlobSidecarsRLP fix: some bugs for multi database Jul 5, 2024
flywukong
flywukong previously approved these changes Jul 8, 2024
View reviewed changes
Copy link
Contributor

@flywukong flywukong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

buddh0
buddh0 previously approved these changes Jul 10, 2024
View reviewed changes
@jingjunLi jingjunLi force-pushed the fix-multidatabase-sidebars branch 2 times, most recently from 62ee8af to 0aafc91 Compare July 11, 2024 09:35
RenRick
RenRick previously approved these changes Jul 16, 2024
View reviewed changes
Copy link

@RenRick RenRick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

core/rawdb/accessors_chain.go Outdated
@@ -34,6 +34,15 @@ import (
"golang.org/x/exp/slices"
)

// In BEP-365: Support Multi-Database Based on Data Pattern, the Chaindata will be divided into three stores: BlockStore, TrieStore, and Others,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multi-Database is not a BEP, pls delete "In BEP-365:"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

@jingjunLi jingjunLi changed the title fix: some bugs for multi database fix: the bug of blobsidecars and downloader with multi-database Jul 16, 2024
RenRick
RenRick previously approved these changes Jul 16, 2024
View reviewed changes
Copy link

@RenRick RenRick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

zzzckck
zzzckck reviewed Jul 16, 2024
View reviewed changes
core/rawdb/accessors_chain.go Outdated
@@ -34,6 +34,15 @@ import (
"golang.org/x/exp/slices"
)

// Support Multi-Database Based on Data Pattern, the Chaindata will be divided into three stores: BlockStore, TrieStore, and Others,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TrieStore -> StateStore?
Others -> ChainStore?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed comment

zzzckck
zzzckck reviewed Jul 16, 2024
View reviewed changes
node/node.go Show resolved Hide resolved
zzzckck
zzzckck previously approved these changes Jul 16, 2024
View reviewed changes
RenRick
RenRick approved these changes Jul 16, 2024
View reviewed changes
Copy link

@RenRick RenRick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zzzckck zzzckck merged commit 87e622e into bnb-chain:develop Jul 16, 2024
7 checks passed
This was referenced Jul 17, 2024
Merged
Merged
flywukong added a commit to flywukong/bsc that referenced this pull request Jul 23, 2024
@flywukong
Revert "fix: the bug of blobsidecars and downloader with multi-databa…
1fc1e35 
…se (bnb-chain#2564)"

This reverts commit 87e622e.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zzzckck zzzckck zzzckck approved these changes

@RenRick RenRick RenRick approved these changes

@flywukong flywukong flywukong left review comments

@buddh0 buddh0 buddh0 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jingjunLi @buddh0 @flywukong @RenRick @zzzckck